Skip to content

Conversation

@johnbillion
Copy link
Owner

@johnbillion johnbillion commented Nov 5, 2024

This introduces password pre-hashing in order to retain the entropy of passwords greater than 72 bytes in size.

  • The wp- prefix is needed in order to differentiate between passwords hashed using this mechanism by WordPress core and a password hashed using vanilla bcrypt via one of the several existing plugins that implement bcrypt. Not doing so would mean not being able to support and upgrade passwords hashed by one of those plugins.
  • The base64 encoding is required to prevent a null byte being present in the hashed password value.
  • sha384 is used because it results in a string 64 bytes in size being passed to bcrypt. This is functionally no different to truncating a sha512 hash.
  • Reinstates the 4096 byte length limit that exists in the phpass implementation currently used in WordPress core.

See WordPress#7333 for full details.

Supporting references

Tickets

Trac ticket: https://core.trac.wordpress.org/ticket/21022
Trac ticket: https://core.trac.wordpress.org/ticket/50027

@github-actions
Copy link

github-actions bot commented Nov 5, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props johnbillion.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@johnbillion
Copy link
Owner Author

johnbillion commented Dec 12, 2024

@soatok @dd32 @Synchro @pkevan Thanks for your input on WordPress#7333 .

This is the linked PR that I wrote at the same time which implements pre-hashing in order to avoid the 72 byte length limit of bcrypt. I've detailed in the description above why a prefix is needed on the resulting hash.

I've just updated it to use hash_hmac instead of manually prepending a prefix on the password value based on the info from @soatok.

If we were to go ahead with pre-hashing, what are your thoughts on this approach?

@soatok
Copy link

soatok commented Dec 12, 2024

LGTM. This would satisfy the baseline requirements I mentioned in the other thread.

While there may be some additional security benefit to having another constant added to https://api.wordpress.org/secret-key/1.1/salt/ and then used in the HMAC step, it would also require admins to manage those salts (e.g., when migrating or restoring from backup). This PR balances a minimal operational load with disarming bcrypt's footgun.

@calvinalkan
Copy link

More:

https://paragonie.com/blog/2016/02/how-safely-store-password-in-2016#why-bcrypt

@johnbillion johnbillion merged commit 1cebd80 into 21022-bcrypt Jan 6, 2025
16 checks passed
@johnbillion johnbillion deleted the 21022-bcrypt-sha2 branch January 7, 2025 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants